Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

security: call /usr/libexec/fips-setup-helper #5928

Merged
merged 1 commit into from
Nov 1, 2024
Merged

security: call /usr/libexec/fips-setup-helper #5928

merged 1 commit into from
Nov 1, 2024

Conversation

anurag03
Copy link

@anurag03 anurag03 commented Oct 10, 2024 •
edited by KKoukiou
Loading

crypto-policies now ships a helper for anaconda to call
in order to just "do the right thing"
and make it not anaconda's responsibility.

Cherry-picked from master commit 123d819
Resolves: RHEL-58667

@pep8speaks
Copy link

pep8speaks commented Oct 10, 2024 •
edited
Loading

Hello @anurag03! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-11-01 07:06:17 UTC

KKoukiou
KKoukiou requested changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the commit message: See 56b0c7c other commits like this one for reference.

Just cherry-pick with git cherry-pick the upstream commit, and mention in the commit message like in the commit linked the Resolves and Cherry-pick lines.

KKoukiou
KKoukiou reviewed Oct 10, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we dont generally sign off commits.
Feel free to leave it if you want to.

tests/unit_tests/pyanaconda_tests/modules/security/test_module_security.py
@@ -1038,7 +1038,7 @@ def test_configure_fips_task(self, mock_util):
task.run()

mock_util.execWithRedirect.assert_called_once_with(
"/usr/libexec/fips-setup-helper",
"/usr/libexec/fips-setup-helper",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please cherry-pick the original commit and you will not need this fixup.

@anurag03 anurag03 force-pushed the ansinha/crypto-policies-helper branch from e25cb46 to 69fe748 Compare October 17, 2024 10:07
KKoukiou
KKoukiou requested changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have typo in the commit message.

cyrpto and then a trailing comma, please fix.

Also please use the following convention for linking the RHEL bug.

Related: RHEL-58667

KKoukiou
KKoukiou requested changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally please just cherry-pick the commit as requested.
This will fix the title of the commit message to be the same like the original.

@anurag03
Copy link
Author

anurag03 commented Oct 17, 2024 •
edited
Loading

This is related to crypto policy helper changes,
cherry-picked from master branch commit: anurag03@123d819
Related: RHEL-58667

@anurag03 anurag03 force-pushed the ansinha/crypto-policies-helper branch from 69fe748 to a4f2052 Compare October 17, 2024 12:35
KKoukiou
KKoukiou requested changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there. It's missing the:

Cherry-picked from commit: ...
Related to: RHEL-*

lines.

So how we do backports is:

  • Cherry-pick the commit from master branch with git cherry-pick
  • Amend the commit message with git commit --am to contain the two information pointed as missing above.
    Please fix these and it's good to go.

@KKoukiou KKoukiou changed the title [RFR]Copied cyrpto policy helper changes to RHEL 9 branch security: call /usr/libexec/fips-setup-helper Oct 17, 2024
@KKoukiou
Copy link
Contributor

This is related to crypto policy helper changes cherry-picked from master branch commit: anurag03@123d819 Related: [RHEL-58667]

This is not the original commit. Please point the original commit. This is your fork.

@anurag03 anurag03 force-pushed the ansinha/crypto-policies-helper branch 3 times, most recently from 128ecc7 to 726dac9 Compare October 24, 2024 10:26
@t184256 @KKoukiou
security: call /usr/libexec/fips-setup-helper
7c89459 
crypto-policies now ships a helper for anaconda to call
in order to just "do the right thing"
and make it not anaconda's responsibility.

Cherry-picked from master commit 123d819
Resolves: RHEL-58667
@KKoukiou KKoukiou force-pushed the ansinha/crypto-policies-helper branch from 726dac9 to 7c89459 Compare November 1, 2024 07:06
KKoukiou
KKoukiou reviewed Nov 1, 2024
View reviewed changes
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anurag03 I force pushed with the suggested commit message changes.

@M4rtinK M4rtinK added the ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). label Nov 1, 2024
@M4rtinK
Copy link
Contributor

M4rtinK commented Nov 1, 2024

/kickstart-test --testtype smoke

@M4rtinK M4rtinK merged commit 6ca7b14 into rhinstaller:rhel-9 Nov 1, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KKoukiou KKoukiou KKoukiou approved these changes

Assignees
No one assigned
Labels
ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). rhel-9
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@anurag03 @pep8speaks @KKoukiou @M4rtinK @t184256